Skip to content

Conversation

daveverwer
Copy link
Member

Merge after #3609

Adds a simple uptime check route that exercises the database that we should monitor with the Azure and StatusPage checkers.

I'll set this page to cache this for 5 minutes which will prevent it being abused, but it will be a much more accurate test than the home page, which is cached for many hours.

owner: "SwiftPackageIndex",
repository: "SemanticVersion")
return "Success"
}
Copy link
Member

@finestructure finestructure Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our smoke test is already exercising the db via package page and search requests. I'm wondering why we'd need this extra route?

If we do this, I should call it /healthcheck.

Edit: If we do this, I think we should call it /healthcheck.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the proposal for these changes I offered you two alternatives for this piece and you expressed no opinion between them.

You had previously expressed an opinion to not get misleading data into our analytics, which I agree with, and this scenario avoids that completely as it allows us to keep the automated traffic detection protection enabled for the home page.

But of course it’s wrong. Why would I expect anything different?

Copy link
Member Author

@daveverwer daveverwer Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, and you know this already, the smoke test only happens at deploy time, the uptime check is a constant monitor.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite, the smoke tests also run every four hours. As to the Azure probes, they can probe a package page in addition if we feel that's important. I'm simply asking it wouldn't be simpler and more realistic to do just that.

I'll tick the PR either way, it was just a question if you've considered this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd meant to tick the PR earlier but forgot after commenting.

@finestructure
Copy link
Member

finestructure commented Jan 16, 2025

FWIW, we know that the CF cache isn't preventing the uptime checker from alerting. I don't think we need a special route with special caching.

@daveverwer daveverwer enabled auto-merge January 16, 2025 10:54
@daveverwer daveverwer merged commit 34df242 into main Jan 16, 2025
6 checks passed
@daveverwer daveverwer deleted the uptime-check branch January 16, 2025 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants